Add PWA option to ensure cross-origin isolation headers on web export #86089

adamscott · 2023-12-12T20:53:21Z

Sister PR to #85939

When the PWA option is enabled, the installed PWA will make sure to enable COEP/COOP request headers if the option is selected.

platform/web/export/export_plugin.cpp

platform/web/doc_classes/EditorExportPlatformWeb.xml

Faless

Thanks for looking into this 💙 .
It's looking good overall 🚀 (see my comment on moving some code into engine.js).
I like the s/@/__ rename, I wish it was done in its own PR though (EDIT: We can probably refactor this further into something similar to GODOT_CONFIG in a separate PR).
There are some "style" changes (e.g. using the loose equality operator) which are not great in JS .
I'll look into normalizing the service worker using eslint like we do for other bits of the javascript.

Faless · 2023-12-15T11:59:43Z

misc/dist/html/full-size.html

+		if (GODOT_CONFIG['serviceWorker'] && GODOT_CONFIG['ensureCrossOriginIsolationHeaders'] && 'serviceWorker' in navigator) {
+			// There's a chance that installing the service worker would fix the issue
+			Promise.race([
+				navigator.serviceWorker.getRegistration().then((registration) => {
+					if (registration != null) {
+						return Promise.reject(new Error('Service worker already exists.'));
+					}
+					return registration;
+				}).then(() => engine.installServiceWorker()),
+				// For some reason, `getRegistration()` can stall
+				new Promise((resolve) => {
+					setTimeout(() => resolve(), 2000);
+				}),
+			]).catch((err) => {
+				console.error('Error while registering service worker:', err);
+			}).then(() => {
+				window.location.reload();
+			});
+		} else {
+			// Display the message as usual
+			const missingMsg = 'Error\nThe following features required to run Godot projects on the Web are missing:\n';
+			displayFailureNotice(missingMsg + missing.join('\n'));
+		}


I think we should move this to a function of platform/js/engine/engine.js like we did for Engine.getMissingFeatures.

I'll keep the code as is, finally. We would need to pass in parameters to that function the config and the engine variable, which seems a little bit excessive.

Faless · 2023-12-15T12:53:03Z

Attached the 2 patches (001 adds the service worker linting and its config, 002 is just a linter pass).
patch-001.diff.txt
patch-002.diff.txt

EDIT: Remember to git add platform/web/.eslintrc.sw.js

adamscott · 2023-12-15T16:58:34Z

There are some "style" changes (e.g. using the loose equality operator) which are not great in JS .

Actually, this was done precisely with that in mind. null is pretty much the exception when using the loose equality operator. (see https://eslint.org/docs/latest/rules/eqeqeq#smart)

You actually want to compare a variable loosely to null. That's because Javascript considers null and undefined nullish (hence both values pass the loose equality test with null), but not the falsy values (eg. "", 0, which do not pass the loose equality test with null).

Hence why, instead, we should replace all the if (value) tests by value != null and if (!value) tests by value == null. (but that's for another PR)

undefined == null; // `true`
"" == null; // `false`

adamscott · 2023-12-15T17:05:41Z

Attached the 2 patches (001 adds the service worker linting and its config, 002 is just a linter pass). patch-001.diff.txt patch-002.diff.txt

EDIT: Remember to git add platform/web/.eslintrc.sw.js

Done!

platform/web/package.json

Faless

Needs a rebase (tested here: https://github.com/Faless/godot/tree/temp/86089) and squash, then I think this is good to go.
Great job 👍

adamscott · 2024-02-12T16:03:24Z

Rebase done!

akien-mga · 2024-02-12T22:36:30Z

Thanks! Great feature to have :)

adamscott requested review from a team as code owners December 12, 2023 20:53

adamscott changed the title ~~Add option to ensure cross-origin isolation headers on web export~~ Add PWA option to ensure cross-origin isolation headers on web export Dec 12, 2023

adamscott added enhancement platform:web labels Dec 12, 2023

adamscott added this to the 4.x milestone Dec 12, 2023

AThousandShips reviewed Dec 12, 2023

View reviewed changes

platform/web/export/export_plugin.cpp Outdated Show resolved Hide resolved

adamscott force-pushed the pwa-coop-coep branch 3 times, most recently from cd0c35e to ba6cb81 Compare December 12, 2023 21:16

adamscott commented Dec 13, 2023

View reviewed changes

platform/web/doc_classes/EditorExportPlatformWeb.xml Show resolved Hide resolved

adamscott marked this pull request as draft December 14, 2023 15:52

This comment was marked as outdated.

Sign in to view

adamscott marked this pull request as ready for review December 14, 2023 16:52

adamscott requested a review from Faless December 14, 2023 16:53

adamscott force-pushed the pwa-coop-coep branch 2 times, most recently from 830601c to ad8b31d Compare December 14, 2023 18:05

Faless reviewed Dec 15, 2023

View reviewed changes

platform/web/package.json Outdated Show resolved Hide resolved

Faless reviewed Dec 15, 2023

View reviewed changes

platform/web/package.json Outdated Show resolved Hide resolved

Faless reviewed Dec 15, 2023

View reviewed changes

platform/web/package.json Show resolved Hide resolved

Faless approved these changes Feb 6, 2024

View reviewed changes

akien-mga added the topic:export label Feb 6, 2024

akien-mga modified the milestones: 4.x, 4.3 Feb 6, 2024

adamscott force-pushed the pwa-coop-coep branch 2 times, most recently from 95a68e5 to b484cd5 Compare February 12, 2024 14:27

adamscott force-pushed the pwa-coop-coep branch from b484cd5 to 0b13127 Compare February 12, 2024 14:27

Add option to ensure cross-origin isolation headers on web export

62cec03

adamscott force-pushed the pwa-coop-coep branch from 0b13127 to 62cec03 Compare February 12, 2024 14:27

akien-mga merged commit 1b55fa1 into godotengine:master Feb 12, 2024
16 checks passed

adamscott mentioned this pull request Oct 2, 2024

Godot Web Build reloads infinitely on github.io #97580

Closed

adamscott mentioned this pull request Nov 6, 2024

[Web] Ensure editor crossorigin isolation headers #98901

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PWA option to ensure cross-origin isolation headers on web export #86089

Add PWA option to ensure cross-origin isolation headers on web export #86089

adamscott commented Dec 12, 2023

This comment was marked as outdated.

Faless left a comment •

edited

Loading

Faless Dec 15, 2023

This comment was marked as outdated.

adamscott Feb 12, 2024

Faless commented Dec 15, 2023 •

edited

Loading

adamscott commented Dec 15, 2023 •

edited

Loading

adamscott commented Dec 15, 2023

Faless left a comment

adamscott commented Feb 12, 2024

akien-mga commented Feb 12, 2024

Add PWA option to ensure cross-origin isolation headers on web export #86089

Add PWA option to ensure cross-origin isolation headers on web export #86089

Conversation

adamscott commented Dec 12, 2023

This comment was marked as outdated.

Faless left a comment • edited Loading

Choose a reason for hiding this comment

Faless Dec 15, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

adamscott Feb 12, 2024

Choose a reason for hiding this comment

Faless commented Dec 15, 2023 • edited Loading

adamscott commented Dec 15, 2023 • edited Loading

adamscott commented Dec 15, 2023

Faless left a comment

Choose a reason for hiding this comment

adamscott commented Feb 12, 2024

akien-mga commented Feb 12, 2024

Faless left a comment •

edited

Loading

Faless commented Dec 15, 2023 •

edited

Loading

adamscott commented Dec 15, 2023 •

edited

Loading